chore(test): migrate from Karma to Jest and add CI jobs and mappings#6429
chore(test): migrate from Karma to Jest and add CI jobs and mappings#6429
Conversation
gnbm
changed the title
…ent, typed filters
johnjenkins
left a comment
johnjenkins
left a comment
There was a problem hiding this comment.
Love the pipeline icons 😄
Just one question
test/bundler/jest-dom-utils.ts
Outdated
| }; | ||
| window.addEventListener('appload', onAppLoad); | ||
| // if app already loaded synchronously, resolve on next tick | ||
| setTimeout(() => resolve(), 0); |
There was a problem hiding this comment.
how does this 'know' that the app loaded synchronously here sorry?
Won't this just resolve every time after a tick?
There was a problem hiding this comment.
Great question! This is actually a bug in the code. You're absolutely right - the setTimeout(() => resolve(), 0) will always resolve after one tick, regardless of whether the app actually loaded or not.
Will look into this asap
|
@gnbm - just to clarify ... these tests used to run in an actual browser and now they run in ... jsdom(?) |
Co-authored-by: John Jenkins <johnljenkins@Hotmail.com>
The previous implementation had a setTimeout with 0ms that would always resolve immediately, making the 'appload' event listener effectively useless. This could cause tests to continue before the app was actually ready. This fix: - Adds a resolved flag to prevent double resolution - Changes timeout from 0ms to 5000ms (reasonable fallback) - Only resolves via timeout if the event hasn't fired - Adds warning message when falling back to timeout - Properly cleans up event listener in both paths
Now that I'm thinking better about this, I probably should have migrated to Playwright instead, since with this approach, even though the tests would run faster, we'll lose a lot of things about real browsers. I'll close the PR and look into this from a different perspective. Thank you for the review and for raising all of this, since I still don't have enough context and was just trying to start contributing with some AI help to accelerate the process. |
Absolutely no problem - appreciate you trying to tidy this up! |
Will explore that option first. Thanks for sharing and for your amazing contribution to this great project |
2 participants
What is the current behavior?
GitHub Issue Number: N/A
What is the new behavior?
test/bundler/jest-dom-utils.ts(jsdom loader that executes built module scripts and waits for appload/componentOnReady).test/bundler/vite-bundle-test/vite-bundle.spec.tsto Jest and pointed it to the built index.html.test/bundler/karma.config.ts,karma-stencil-utils.ts, andupdated test/bundler/package.json.test/bundler/jest.config.js.ci.test: run full repo Jest + bundler sub-config.CI:
.github/workflows/ci.ymlwith separate jobs:Documentation
N/A
Does this introduce a breaking change?
Testing
npm run test.jestnpm run test.bundlerOther information
Impact:
npm run ci.test(or each job separately).Notes: The core suite is green. Occasional “worker failed to exit gracefully” warnings persist but are benign.